Skip to content

Consistently handle undeliverable exceptions in RxJava and Reactor in… #1638

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 5, 2019

Conversation

qwwdfsad
Copy link
Collaborator

@qwwdfsad qwwdfsad commented Oct 29, 2019

…tegrations

Use tryOnError in RxJava to make exception delivery check-and-act race free.
Deliver undeliverable exceptions via RxJavaPlugins instead of handleCoroutineException.
This is a deliberate choice for a multiple reasons:
* When using Rx (whether with coroutines or not), undeliverable exceptions are inevitable and users should hook into RxJavaPlugins anyway. We don't want to force them using Rx-specific CoroutineExceptionHandler all over the place
* Undeliverable exceptions provide additional helpful stacktrace and proper way to distinguish them from other unhandled exceptions
* Be consistent with reactor where we don't have try*, thus cannot provide a completely consistent experience with CEH (at least, without wrapping all the subscribers)

Do the similar in Reactor integration, but without try*, Reactor does not have notion of undeliverable exceoptions and handles them via Operators.* on its own.

Also, get rid of ASCII tables that does not properly render in IDEA

Fixes #252
Fixes #1614

@qwwdfsad
Copy link
Collaborator Author

Can't say I am excited or happy about this fix, but it looks like the lesser evil

@qwwdfsad qwwdfsad requested a review from elizarov October 29, 2019 17:11
Copy link
Contributor

@elizarov elizarov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Good to go.

@qwwdfsad qwwdfsad force-pushed the rx-async-cancellation branch from e091a64 to b420e38 Compare December 5, 2019 15:59
…tegrations

Use tryOnError in RxJava to make exception delivery check-and-act race free.
Deliver undeliverable exceptions via RxJavaPlugins instead of handleCoroutineException.
This is a deliberate choice for a multiple reasons:
  * When using Rx (whether with coroutines or not), undeliverable exceptions are inevitable and users should hook into RxJavaPlugins anyway. We don't want to force them using Rx-specific CoroutineExceptionHandler all over the place
  * Undeliverable exceptions provide additional helpful stacktrace and proper way to distinguish them from other unhandled exceptions
  * Be consistent with reactor where we don't have try*, thus cannot provide a completely consistent experience with CEH (at least, without wrapping all the subscribers)\

Do the similar in Reactor integration, but without try*, Reactor does not have notion of undeliverable exceoptions and handles them via Operators.* on its own.

Also, get rid of ASCII tables that are not properly render in IDEA

Fixes #252
Fixes #1614
@qwwdfsad qwwdfsad force-pushed the rx-async-cancellation branch from b420e38 to 556c92f Compare December 5, 2019 16:33
@qwwdfsad qwwdfsad merged commit a930b0c into develop Dec 5, 2019
@qwwdfsad qwwdfsad deleted the rx-async-cancellation branch December 5, 2019 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants